Skip to content

Conversation

mshockwave
Copy link
Member

There are many cases where SDNodes shown in the -debug-only=isel/isel-dump of a DAG -- especially a big DAG -- just randomly spread everywhere, which can be really really painful to read sometimes.
This patch adds a flag to sort DAGs before printing them in those debug prints. Note that sorted DAGs might have different schedulings and hence different results compared to the original one, but I thought it's still useful for debugging many of the (codegen) cases.

@mshockwave mshockwave requested review from RKSimon, arsenm and topperc July 20, 2025 19:34
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Jul 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 20, 2025

@llvm/pr-subscribers-llvm-selectiondag

Author: Min-Yih Hsu (mshockwave)

Changes

There are many cases where SDNodes shown in the -debug-only=isel/isel-dump of a DAG -- especially a big DAG -- just randomly spread everywhere, which can be really really painful to read sometimes.
This patch adds a flag to sort DAGs before printing them in those debug prints. Note that sorted DAGs might have different schedulings and hence different results compared to the original one, but I thought it's still useful for debugging many of the (codegen) cases.


Full diff: https://github.com/llvm/llvm-project/pull/149732.diff

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (+24-10)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 26071ed70c9db..4964edd620e47 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -144,6 +144,12 @@ UseMBPI("use-mbpi",
         cl::init(true), cl::Hidden);
 
 #ifndef NDEBUG
+static cl::opt<bool>
+    SortDAGBeforeDump("sort-dags-before-isel-dump", cl::Hidden,
+                      cl::desc("Sort SelectionDAGs before showing them in"
+                               " debug prints during SelectionDAGISel."),
+                      cl::init(false));
+
 static cl::opt<std::string>
 FilterDAGBasicBlockName("filter-view-dags", cl::Hidden,
                         cl::desc("Only display the basic block whose name "
@@ -903,6 +909,14 @@ void SelectionDAGISel::ComputeLiveOutVRegInfo() {
   } while (!Worklist.empty());
 }
 
+#ifndef NDEBUG
+static void dumpSelectionDAG(SelectionDAG *DAG) {
+  if (SortDAGBeforeDump)
+    DAG->AssignTopologicalOrder();
+  DAG->dump();
+}
+#endif
+
 void SelectionDAGISel::CodeGenAndEmitDAG() {
   StringRef GroupName = "sdag";
   StringRef GroupDescription = "Instruction Selection and Scheduling";
@@ -930,7 +944,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   ISEL_DUMP(dbgs() << "\nInitial selection DAG: "
                    << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                    << "'\n";
-            CurDAG->dump());
+            dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (TTI->hasBranchDivergence())
@@ -950,7 +964,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   ISEL_DUMP(dbgs() << "\nOptimized lowered selection DAG: "
                    << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                    << "'\n";
-            CurDAG->dump());
+            dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (TTI->hasBranchDivergence())
@@ -972,7 +986,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   ISEL_DUMP(dbgs() << "\nType-legalized selection DAG: "
                    << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                    << "'\n";
-            CurDAG->dump());
+            dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (TTI->hasBranchDivergence())
@@ -996,7 +1010,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
     ISEL_DUMP(dbgs() << "\nOptimized type-legalized selection DAG: "
                      << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                      << "'\n";
-              CurDAG->dump());
+              dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
     if (TTI->hasBranchDivergence())
@@ -1014,7 +1028,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
     ISEL_DUMP(dbgs() << "\nVector-legalized selection DAG: "
                      << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                      << "'\n";
-              CurDAG->dump());
+              dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
     if (TTI->hasBranchDivergence())
@@ -1030,7 +1044,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
     ISEL_DUMP(dbgs() << "\nVector/type-legalized selection DAG: "
                      << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                      << "'\n";
-              CurDAG->dump());
+              dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
     if (TTI->hasBranchDivergence())
@@ -1050,7 +1064,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
     ISEL_DUMP(dbgs() << "\nOptimized vector-legalized selection DAG: "
                      << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                      << "'\n";
-              CurDAG->dump());
+              dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
     if (TTI->hasBranchDivergence())
@@ -1070,7 +1084,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   ISEL_DUMP(dbgs() << "\nLegalized selection DAG: "
                    << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                    << "'\n";
-            CurDAG->dump());
+            dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (TTI->hasBranchDivergence())
@@ -1090,7 +1104,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   ISEL_DUMP(dbgs() << "\nOptimized legalized selection DAG: "
                    << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                    << "'\n";
-            CurDAG->dump());
+            dumpSelectionDAG(CurDAG));
 
 #if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (TTI->hasBranchDivergence())
@@ -1114,7 +1128,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   ISEL_DUMP(dbgs() << "\nSelected selection DAG: "
                    << printMBBReference(*FuncInfo->MBB) << " '" << BlockName
                    << "'\n";
-            CurDAG->dump());
+            dumpSelectionDAG(CurDAG));
 
   if (ViewSchedDAGs && MatchFilterBB)
     CurDAG->viewGraph("scheduler input for " + BlockName);

@RKSimon
Copy link
Collaborator

RKSimon commented Jul 21, 2025

Or we could focus efforts on finally ensuring the DAG is topologically sorted to begin with: #83422

@mshockwave
Copy link
Member Author

Or we could focus efforts on finally ensuring the DAG is topologically sorted to begin with: #83422

True, aside from the problems raised in the said issue, I remember compilation time is/was also a concern?

But in any case, I feel like this patch can provide a quick and easy solution for improving developers' life, at least in a short term.

@RKSimon
Copy link
Collaborator

RKSimon commented Jul 21, 2025

Or we could focus efforts on finally ensuring the DAG is topologically sorted to begin with: #83422

True, aside from the problems raised in the said issue, I remember compilation time is/was also a concern?

But in any case, I feel like this patch can provide a quick and easy solution for improving developers' life, at least in a short term.

Agreed, I'm just rather annoyed at the lack on interest in helping with something that should have been done a decade ago.....

FTR a topological sorted dag should reduce backend compile time: https://llvm-compile-time-tracker.com/?config=Overview&stat=instructions%3Au&remote=RKSimon

@mshockwave
Copy link
Member Author

Close in favor of #161097

@mshockwave mshockwave closed this Sep 28, 2025
mshockwave added a commit that referenced this pull request Oct 3, 2025
An alternative approach to #149732 , which sorts the DAG before dumping
it. That approach runs a risk of altering the codegen result as we don't
know if any of the downstream DAG users relies on the node ID, which was
updated as part of the sorting.

The new method proposed by this PR does not update the node ID or any of
the DAG's internal states: the newly added
`SelectionDAG::getTopologicallyOrderedNodes` is a const member function
that returns a list of all nodes in their topological order.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants